-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jenkins 13493 - Automatic git cache maintenance #1277
base: master
Are you sure you want to change the base?
Conversation
Thank You @MarkEWaite for fixing those tests. |
|
||
public static String checkSanity(String cron) throws ANTLRException { | ||
try { | ||
CronTab cronTab = new CronTab(cron.trim()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can the input string cron
ever be null? It could be a potential NPE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cron string will not be null. If the user leave the text field blank in the UI, an empty string is passed to this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a NonNull annotation to declare to spotbugs that the argument should never be allowed to be null.
src/main/java/jenkins/plugins/git/maintenance/MaintenanceUI.java
Outdated
Show resolved
Hide resolved
src/main/java/jenkins/plugins/git/maintenance/MaintenanceUI.java
Outdated
Show resolved
Hide resolved
src/main/java/jenkins/plugins/git/maintenance/TaskExecutor.java
Outdated
Show resolved
Hide resolved
|
||
List<Task> configuredTasks = config.getMaintenanceTasks(); | ||
addTasksToQueue(configuredTasks); | ||
// Option of Using the same thread for executing more maintenance task, or create a new thread the next minute and execute the maintenance task. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting, would like to discuss this more in our weekly sync.
|
||
public static String checkSanity(String cron) throws ANTLRException { | ||
try { | ||
CronTab cronTab = new CronTab(cron.trim()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a NonNull annotation to declare to spotbugs that the argument should never be allowed to be null.
src/main/java/jenkins/plugins/git/maintenance/MaintenanceUI.java
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,3 @@ | |||
<div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MarkEWaite needs to find the technique to add the help icon to the user interface. Users will expect to view help for the various maintenance commands.
@Hrushi20 it appears there are testing failing in the CI job that were not failing before. I'm on vacation and won't be able to investigate for several weeks. If you have time over the next few weeks, it would be very nice if you could investigate the test failures. |
Sure @MarkEWaite I'll look into it. Enjoy your vacation. |
Hello @MarkEWaite, I have gone through the failed tests. I realized that the below code is not creating a cache on the Jenkins controller.
Before I could access the caches, but now I am not able to create caches. Is there a way to create a MultibranchPipeline so that it handle creating the caches. |
Hello @MarkEWaite, can you help me fix these failing tests. The problem I am facing right now is that I am not able to create fake caches during tests. |
I won't be able to help with these tests until after I've completed the release of git client plugin 4.0.0 and git plugin 5.0.0. Those releases are getting all the time that I can allocate to the git plugin and git client plugin. |
Hey @MarkEWaite, incremental repack is failing. I'm getting task timeout error. Can you help me with that. I fixed all the other tests. Looking forward to getting this feature merged. |
Provides maintenance API
Signed-off-by: Hrushi20 <[email protected]>
Signed-off-by: Hrushi20 <[email protected]>
Signed-off-by: Hrushi20 <[email protected]>
Signed-off-by: Hrushi20 <[email protected]>
2214dbc
to
c0ff128
Compare
Hey @MarkEWaite, I've fixed all the Unit test Issues. I ran a git rebase and the code is up to date with latest master. There are 2 spotbug issues, I am facing. Should I use the Spotbug flag to neglect such an issue? Looking forward to getting this merged. |
Hey @MarkEWaite, can we get this code merged? |
Hey! What are the steps to take to get this PR merged @MarkEWaite? Can I ignore the Spotbugs error? |
JENKINS-13493 - Automatic git cache maintenance
This PR adds git maintenance to the Jenkins controller to optimize the git caches on the controller.
Please refer to Project Page for reference and design:
https://www.jenkins.io/projects/gsoc/2022/projects/automatic-git-cache-maintenance/
Checklist
Types of changes